feat: SessionStore.load_range — cheap head/tail reads for summaries#846
feat: SessionStore.load_range — cheap head/tail reads for summaries#846
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
qing-ant
force-pushed
the
qing/session-store-load-range
branch
from
b0572f8 to
dd336e7
Compare
There was a problem hiding this comment.
All four issues from my earlier review are addressed (head/tail isolation, file_size=None, ReorderingStore override, docstring count) and the bug hunter found nothing new — looks solid, but since this adds a new public SessionStore Protocol method and is stacked on #837, a human should sign off on the API shape.
Extended reasoning...
Overview
This PR adds an optional load_range(key, *, head, tail) method to the SessionStore Protocol so summary derivation (list_sessions_from_store, get_session_info_from_store) can fetch only a head/tail slice instead of full transcripts. Touches types.py (new Protocol method), session_store.py (InMemorySessionStore.load_range), sessions.py (refactors _load_store_entries_as_jsonl → _load_store_session_lite returning a _LiteSessionFile directly; widens _LiteSessionFile.size to int | None), session_store_conformance.py (contract #14), plus a new 347-line test file and updates to three existing spy-store tests.
Prior feedback — all resolved
I left four inline comments on the first revision; commit af09956 addresses each one and adds regression coverage:
- Head/tail concatenation leak → now serialized separately into
_LiteSessionFile(head=…, tail=…); covered bytest_first_prompt_does_not_leak_from_tail. - Misleading
file_size→ nowNoneon the range path; asserted in two tests. ReorderingStoresilently bypassed → now overridesload_rangetoo.- Docstring drift (13→14, optional-methods list) → fixed in both module and function docstrings.
Security risks
None. Read-only summary derivation; no auth, crypto, subprocess, or filesystem-write changes. Inputs are adapter-supplied JSON-safe dicts already trusted by the existing load() path.
Level of scrutiny
Medium. The implementation is clean and the fallback path is unchanged, but this is a public Protocol method addition — an API commitment that third-party adapter authors will implement against. It also changes the default read path for InMemorySessionStore subclasses (which is why three existing tests needed updates). The design follows the established optional-method pattern exactly (raise NotImplementedError default + _store_implements probe), so there's no novelty, but a human should confirm the head=10/tail=20 constants and the tuple[list, list] | None return shape are what we want to commit to.
Other factors
- Stacked on #837, so this should land with/after that PR's review.
- Comprehensive test coverage: routing spies for both presence/absence, conformance contract, overlap/zero/unknown-key edges, and the head/tail-leak regression.
- 643 tests pass per the description; ruff + mypy clean.
…ated, file_size=None); ReorderingStore overrides load_range; bump conformance count to 14
…hold re-append window
qing-ant
force-pushed
the
qing/session-store-load-range
branch
from
6d32464 to
ee2e2c6
Compare
… test_first_prompt_does_not_leak_from_tail to match _LITE_LOAD_HEAD/TAIL_ENTRIES=20/200
|
Moving to draft — exploring a batch |
There was a problem hiding this comment.
All prior feedback addressed (including the test-comment drift in 6d0ca60) and the implementation looks correct — deferring only because this adds a new public SessionStore Protocol method with heuristic constants (head=20/tail=200), which is an API-shape decision worth a human glance.
Extended reasoning...
Overview
This PR adds an optional load_range(key, *, head, tail) method to the public SessionStore Protocol so adapters that support range reads (S3 part-files, Postgres LIMIT, Redis LRANGE) can avoid full-transcript loads when list_sessions_from_store/get_session_info_from_store only need a head/tail slice for summary derivation. Touches types.py (Protocol method), sessions.py (loader refactor + new _LITE_LOAD_HEAD/TAIL_ENTRIES constants), session_store.py (InMemorySessionStore.load_range), the conformance suite (contract #14), and ~430 lines of new tests.
Review history
I reviewed this through three iterations:
- Round 1 flagged a silent test-coverage regression (
ReorderingStorenot overridingload_range), stale 13→14 docstring counts, and a misleadingfile_sizevalue on the range path. - Round 2 flagged a correctness bug where head+tail concatenation let
first_promptleak from tail entries. - Round 3 flagged comment-only drift in a test after the constants were bumped to 20/200.
All were addressed (af09956, 1e7b75c, ee2e2c6, 6d0ca60). The current revision is clean — bug hunting found nothing, and I verified the test comments now reference the constants by name.
Security risks
None. This is read-path optimization for session metadata; no auth, no untrusted-input parsing beyond what already exists, no new write paths.
Level of scrutiny
Medium. The implementation is internal and well-tested, but the Protocol method signature and the head=20/tail=200 constants are public-facing design choices. The constants are justified in the PR body (CLI re-appends metadata on a 32 KB byte threshold ≈ ~160 small entries; preamble observed up to index 14), and the degradation behavior when the slice misses is tested — but a maintainer with CLI-transcript domain knowledge should confirm those numbers are reasonable long-term.
Other factors
The new method follows the same opt-in pattern as the three existing optional Protocol methods (list_sessions/delete/list_subkeys), the conformance suite covers it, and this is stacked on #837 which established the pattern. No CODEOWNERS file in this repo. 648 tests pass per the PR body.
…ist-all (#847) ## Problem `list_sessions_from_store()` calls `store.list_sessions()` for IDs, then `store.load()` **per session** to derive title/first-prompt/branch/etc. With N sessions that's N full-transcript round-trips just to render a list. #846 (`load_range`) cut bytes-per-session but not round-trips — 500 sessions still meant 500+ store calls. ## Public API change — additive only, no breaking changes ```python # New TypedDict (3 fields; `data` is opaque, stores persist verbatim) class SessionSummaryEntry(TypedDict): session_id: str mtime: int data: dict[str, Any] # New helper — adapters call this from append() def fold_session_summary( prev: SessionSummaryEntry | None, key: SessionKey, entries: list[SessionStoreEntry], ) -> SessionSummaryEntry: ... # New optional Protocol method (default: raise NotImplementedError) class SessionStore(Protocol): async def list_session_summaries(self, project_key: str) -> list[SessionSummaryEntry]: ... ``` **Unchanged:** `SessionStore.{append,load,list_sessions,delete,list_subkeys}`, `list_sessions_from_store()` signature/return, `get_session_info_from_store()`, `SDKSessionInfo`, `SessionStoreListEntry`, `InMemorySessionStore` public methods. Stores that don't implement `list_session_summaries` work exactly as before. ## Approach Every field the list view needs is **append-incremental**: 4 are set-once from the first entries (`is_sidechain`, `created_at`, `cwd`, `first_prompt`), 7 are last-write-wins from the tail (`custom_title`, `ai_title`, `last_prompt`, `summary_hint`, `git_branch`, `tag`, `mtime`), 0 require a full scan. So a store can maintain a small summary record alongside each session, updated inside `append()` with the entries already in hand — no re-reads. This PR adds: - **`SessionSummaryEntry`** (TypedDict) — 3-field record (`session_id`, `mtime`, opaque `data`). Stores persist it verbatim and never interpret `data`. - **`fold_session_summary(prev, key, entries)`** — pure helper that folds new entries into the previous summary. Adapters call this from `append()` so derivation logic lives in one place (no per-adapter drift). `created_at` latches the first *parseable* entry timestamp — a documented divergence from the lite-parse path only when the very first entry lacks a timestamp (never happens in CLI-produced transcripts). - **`SessionStore.list_session_summaries(project_key)`** — optional Protocol method returning all summaries for a project in one call. - **Fast path in `list_sessions_from_store()`** — when the store implements `list_session_summaries`: build a unified slot list (summary-derived slots + gap-fill placeholders for sessions present in `list_sessions()` but lacking a sidecar), sort by `mtime`, apply `offset`/`limit`, then `load()` only the placeholders that landed in the page. Summary-backed sidechain/empty sessions are pre-filtered *before* pagination so they don't consume page positions (matching disk/slow-path filter-then-paginate); only gap-fill placeholders that resolve to `None` after load can short-page, so a store with complete sidecars never short-pages. `load()` count is bounded by page size, not total missing — zero `load()` calls when sidecars are complete. Gap-fill is best-effort: if the store lacks `list_sessions` it's skipped with a debug log. Otherwise falls back to the existing per-session `load()` path (bounded at 16 concurrent). - **`InMemorySessionStore`** reference impl (entry-timestamp-derived `mtime` so fast/slow paths sort on one clock) + conformance contract #14. `get_session_info_from_store()` (single session) is unchanged — full `load()` is fine there. ## Correctness `tests/test_session_summary.py::TestParityWithLiteParse` proves `summary_entry_to_sdk_info(fold_session_summary(...))` produces the same `SDKSessionInfo` as the existing `_parse_session_info_from_lite` batch path on the same entry stream. ## For reviewers - `_internal/session_summary.py` — fold logic, esp. first-prompt skip filter (mirrors `_extract_first_prompt_from_head`) - `_internal/sessions.py` fast-path block — fallback semantics - `types.py` — public surface: `SessionSummaryEntry`, `fold_session_summary`, `list_session_summaries` Supersedes #846. TS port: anthropics/claude-cli-internal#30520.
1 participant
Stacked on #837.
What
Adds an optional
load_range(key, *, head, tail)method to theSessionStoreProtocol that returns only the firstheadand lasttailentries of a transcript.Why
list_sessions_from_store()andget_session_info_from_store()currently call fullstore.load()on every session just to derive a title / first prompt / tag. For remote backends with multi-MB sessions (S3 egress, Postgres large-row reads, Redis network round-trips) this is expensive — the lite-parser only needs a small head/tail slice.Adapters that can range-read natively (S3 first/last part-file, Postgres
LIMIT, RedisLRANGE) can implementload_rangeto make session listing O(head+tail) per session instead of O(full transcript).How
SessionStore.load_range— optional Protocol method; default raisesNotImplementedError(same opt-in pattern aslist_sessions/delete/list_subkeys)._load_store_session_lite— probes forload_rangevia_store_implements(); uses it when present, otherwise falls back to fullload(). On the range path, head and tail are serialized to separate JSONL strings and packed directly into_LiteSessionFile(head/tail isolated so head-only scans likefirst_promptnever see tail entries;file_size=Nonesince only a slice was fetched).get_session_messages_from_storekeeps using fullload()— it needs the whole chain.InMemorySessionStoreimplementsload_rangeas a list slice.run_session_store_conformancegains contract Fix KeyError: 'cost_usd' for Max subscription users #14 coveringload_range(skipped when the adapter doesn't implement it).Range sizing (head=20 / tail=200)
The CLI re-appends
custom-title/tag/last-promptmetadata on a 32 KB byte threshold, not an entry count — so up to ~160 small entries can accumulate between re-appends. Arename_session_via_storefollowed by a long turn can push the title 100+ entries deep before the next re-append.tail=200covers that window with margin.head=20covers the preamble before the first user message (mode/progress/attachment/system/snapshot entries — observed up to index 14 in real transcripts). When the preamble exceeds the head slice,first_promptisNoneandsummarydegrades tolastPromptfrom the tail rather than dropping the row.No new public exports — Protocol method only.
Tests
tests/test_session_store_load_range.py:InMemorySessionStore.load_rangehead/tail/overlap/zero/unknown-key.list_sessions_from_store/get_session_info_from_storeroute toload_rangewhen implemented and fall back toload()when absent;get_session_messages_from_storestill full-load.file_size is None.first_promptdoes not leak from tail when head is all noise.custom-titleat index 50 followed by 199 small entries (simulating rename → long turn before 32 KB re-append) — title found viatail=200.head=20→first_prompt is Nonebutsummaryfalls through tolastPrompt(row not dropped).Updated two existing
InMemorySessionStore-subclass spy tests (error degradation, concurrency bound) to overrideload_rangesince that's now the read path for InMemorySessionStore.ReorderingStorelikewise overridesload_range.648 passed; ruff + mypy clean.